refactor(llm): derive valid providers and expose direct env reads - #718
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Checked against #673. All four "Done when" items hold. Both Two things I verified beyond that. Behaviour is preserved where it is easiest to lose. I also added the control your mutation run was missing. Deleting all six The part I want to call out is that you noticed all six lines had to go, commented per-provider examples included, because Merging shortly. |
Description
The LLM configuration duplicates its provider list and hides four environment reads from the documentation guard. Derive the provider list from
DEFAULT_MODELSand read the fourLLM_*variables directly, preserving resolution and validation behavior.Type of Change
Related Issue
Closes #673
Changes Made
Object.keys(DEFAULT_MODELS)in provider resolution, validation, and the invalid-provider message.getEnvVarand update the environment guard's coverage-boundary description.Testing
bun run test:unit -t 'environment variable documentation|accepts every provider in DEFAULT_MODELS': 10 passed / 0 failed after the refactor. The provider case accepts every key derived fromObject.keys(DEFAULT_MODELS)and rejects an unknown provider; the new discovery assertion failed before the direct reads were introduced.Mutation proof: temporarily removed all six active/commented
LLM_MODEL=assignments from.env.example, then ranbun run test:unit -t 'environment variable documentation': 8 passed / 1 failed, reportingLLM_MODELas undocumented. All six must be removed because the existing extractor intentionally accepts commented examples. Restored.env.examplebyte-for-byte; it has no diff in this PR.Full Linux validation used the repository's unchanged CI workflow on submitted commit
d87c448832369537d6256f82948556bbd6737b22: run and logs.bun run test:coverage: 14,715 tests passed, all 391 core test files and 34 component isolation groups passed.bun run coverage:check: 46,321 / 46,321 lines (100%).The fork run's overall status is red solely because SonarCloud Analysis lacks the upstream token/project access. All test/build jobs passed. CLAUDE.md explicitly excludes SonarCloud from required checks, and the upstream workflow skips it for fork PRs. Upstream required workflows still need normal maintainer approval.
Full validation ran on GitHub-hosted Linux: this Windows host cannot run the container-based checks because Docker Desktop fails at inference-manager initialization, and its native full component runner encounters SQLite cleanup
EBUSY. No complete native-Windows pass is claimed.Checklist
Additional Notes
AI-assisted implementation, review, and validation. This branch starts independently from
mainand contains only this issue's change.CI follow-up
The fork-run SonarCloud 401 is tracked in #732 and fixed by #733. The inherited condition admitted fork-owned pushes and fork-local PRs to the canonical SonarCloud project. The dedicated CI fix run now succeeds: all nine executable test/build jobs pass, and SonarCloud is scoped to the canonical repository. That run tests CI fix commit
80a318b; this PR's exact-head verification remains the original run linked above, whose nine executable jobs passed. Upstream Actions still await maintainer approval.